Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump to xamarin/Java.Interop/master@fdc200cc #5468

Merged
merged 1 commit into from
Jan 7, 2021

Conversation

jonpryor
Copy link
Member

@jonpryor jonpryor commented Jan 6, 2021

Fixes: dotnet/java-interop#767

Changes: dotnet/java-interop@7574f16...fdc200c

Commit a7413a2 added support for invoking java-source-utils.jar
on @(JavaSourceJar) to extract Javadoc comments and translate them
into C# XML Documentation Comments.

What this can also do is provide correct parameter names.
As of commit a7413a2, the BindingBuildTest.JavaSourceJar()
integration test would emit the warning:

obj/Debug/generated/src/Com.Xamarin.Android.Test.Msbuildtest.JavaSourceJarTest.cs(75,20):
warning CS1572: XML comment has a param tag for 'name', but there is no parameter by that name

Commit dotnet/java-interop@fdc200cc allows java-source-utils.jar
output to be used with class-parse, allowing
@(_JavaSourceJavadocXml) files -- the output of
java-source-utils.jar -- to be included in
@(_AndroidDocumentationPath).

This allows @(JavaSourceJar) files to provide parameter names
within bindings, removing the CS1572 warning, and making for
better overall bindings.

We can see the benefits of this change in
tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs,
which required changes because the parameter names in the Java
DataListener.onDataReceived() method could now be determined;
previously they couldn't, resulting in the P0/P1/etc. names.
With the provision of @(JavaSourceJar) -- a7413a2 updated
Xamarin.Android.McwGen-Tests.csproj to have @(JavaSourceJar) --
the parameter names can now be determined, improving the binding.

Fixes: dotnet/java-interop#767

Changes: dotnet/java-interop@7574f16...fdc200c

  * dotnet/java-interop@fdc200cc: [Xamarin.Android.Tools.Bytecode] Relax _ApiXml check (dotnet#772)
  * dotnet/java-interop@f1b93653: [generator] Change generated code to not emit CA1305 warning. (dotnet#771)
  * dotnet/java-interop@2244407d: [generator] Ensure DIM from assembly refs are correctly marked (dotnet#770)
  * dotnet/java-interop@da73d6a5: [Java.Interop] Prevent premature collection w/ JniInstance* (dotnet#768)

Commit a7413a2 added support for invoking `java-source-utils.jar`
on `@(JavaSourceJar)` to extract Javadoc comments and translate them
into C# XML Documentation Comments.

What this can *also* do is provide correct parameter names.
As of commit a7413a2, the `BindingBuildTest.JavaSourceJar()`
integration test would emit the warning:

	obj/Debug/generated/src/Com.Xamarin.Android.Test.Msbuildtest.JavaSourceJarTest.cs(75,20):
	warning CS1572: XML comment has a param tag for 'name', but there is no parameter by that name

Commit dotnet/java-interop@fdc200cc allows `java-source-utils.jar`
output to be used with `class-parse`, allowing
`@(_JavaSourceJavadocXml)` files -- the output of
`java-source-utils.jar` -- to be included in
`@(_AndroidDocumentationPath)`.

This allows `@(JavaSourceJar)` files to provide parameter names
within bindings, removing the CS1572 warning, and making for
better overall bindings.
@jonpryor jonpryor force-pushed the jonp-use-jsu-for-param-names branch from a0ea50d to 01fcfed Compare January 6, 2021 23:37
Copy link
Contributor

@dellis1972 dellis1972 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks ok. I think we need some release notes though. Will this impact users like it did our test? i.e will they need to update code to get around build errors?

@jonpryor
Copy link
Member Author

jonpryor commented Jan 7, 2021

Will this impact users like it did our test?

This shouldn't actually impact anybody. (Rephrased; it fixes a regression!)

The whole point to @(JavaSourceJar) was to do exactly what is happening here: parse the .java files within @(JavaSourceJar) into "something", which can then be used to get better parameter names.

Before JDK 11 support, before 380e95e, this was done in the _GenerateJavaDocFromSourceJars target: we'd extract @(JavaSourceJar), run javadoc on the sources, pray that it worked -- it did for BindingTests.JavaSourceJar()! -- and if it did work, we'd process the resulting HTML to extract parameter names.

The problem is that JDK 11 changed the HTML output format, breaking our parsing, so to "move forward" we disabled @(JavaSourceJar) when running under JDK 11.

…except we didn't just disable under JDK 11; we disabled everywhere: https://github.com/xamarin/xamarin-android/blob/380e95e340168f68b1ae3eb43e0a71b05bc6d80b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Documentation.targets#L19-L21

^^ always evaluates to "" (unset), because $(_JdkVersion) isn't set at the time that property group is evaluated.

So before JDK 11 support, we'd attempt to process @(JavaSourceJar). After JDK 11 support, we didn't at all, even though we intended to.

Thus, in migrating away from javadoc to java-source-utils.jar, we fixed this inadvertent regression, which apparently nobody noticed.

(…and probably nobody noticed because only 36 people on GitHub use it.)

@jonpryor jonpryor merged commit 0e95ec7 into dotnet:master Jan 7, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generator should pull parameter names from Javadoc
4 participants